Netty: Fix releasing limit for connections pool per host #820
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
You mean Netty, right? |
|
sorry, completely misleading step. I mean in pom.xml from prove-of-fail have just removed it just ignore ;-) |
|
I've removed this step "2.use apache http client" from the pull request description |
|
Fix seems legit, but could you add a test for it, please? |
|
sure, will do. |
|
Thanks! |
slandelle
changed the title
|
Adding the test myself. Thanks a lot for reporting and fixing! |
slandelle
pushed a commit
that referenced
this pull request
Feb 23, 2015
Netty: Fix releasing limit for connections pool per host
slandelle
added a commit
that referenced
this pull request
Feb 23, 2015
|
Awesome! You are fast! ;-)Thanks a lot |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I've found a bug in async-http-client.
1.set 'maxConnections' and 'maxConnectionsPerHost' simultaneously.
2.try to GET the same host multiple times. With channels auto-closed after each request (with no chance here: tryToOfferChannelToPool https://github.com/AsyncHttpClient/async-http-client/blob/async-http-client-1.9.10/src/main/java/com/ning/http/client/providers/netty/channel/ChannelManager.java#L285)
3.Semaphore freeChannelsForHost (here https://github.com/AsyncHttpClient/async-http-client/blob/async-http-client-1.9.10/src/main/java/com/ning/http/client/providers/netty/channel/ChannelManager.java#L142) won't be decremented on channel close and you'll get
"Caused by: java.io.IOException: Too many connections per host XXX"
Because there is no mapping in channelId2KeyPool
(https://github.com/AsyncHttpClient/async-http-client/blob/async-http-client-1.9.10/src/main/java/com/ning/http/client/providers/netty/channel/ChannelManager.java#L140)
I think it should be added here, while "registerOpenChannel" (https://github.com/AsyncHttpClient/async-http-client/blob/async-http-client-1.9.10/src/main/java/com/ning/http/client/providers/netty/channel/ChannelManager.java#L384)
prove-of-fail can be found here https://github.com/kullfar/async-test
steps to reproduce:
git clone 'https://github.com/kullfar/async-test.git'
cd async-test
mvn clean install
java -jar target/async-test-1.0-SNAPSHOT.jar
You will get "Caused by: java.io.IOException: Too many connections per host 3" line shortly
If you'll compile my pull request version and change pom.xml to use 1.9.11-SNAPSHOT version, there will be no Exceptions.
ps. It's the pull request to the 1.9.x branch (just because I use it ritgh now in prod), but as I can see 2.x branch needs to be fixed as well